Use optionalField for creation in ExplainSMPolicy serialization #1507
Conversation
Signed-off-by: Tarun-kishore <[email protected]>
Tarun-kishore
requested review from
Hailong-am,
bowenlan-amzn,
shiv0408,
soosinha,
tandonks and
vikasvb90
as code owners
| metadata?.let { | ||
| builder | ||
| .field(SMMetadata.CREATION_FIELD, it.creation) | ||
| .optionalField(SMMetadata.CREATION_FIELD, it.creation) |
There was a problem hiding this comment.
will this have bwc issue when doing rolling upgrade? guess we need to add version check here
There was a problem hiding this comment.
Yes, Good catch. Added a version check.
There was a problem hiding this comment.
@Hailong-am I added a version check for greater than 3.3 (which is current version). The coverage is failing though, version 3.4 needs to be available for testing it. Rest all CI workflows are passing.
Signed-off-by: Tarun-kishore <[email protected]>
Tarun-kishore
force-pushed
the
bug-fix/explain-serialization
branch
from
b07015e to
5079606
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1507 +/- ##
==========================================
- Coverage 76.42% 76.42% -0.01%
==========================================
Files 375 375
Lines 18835 18838 +3
Branches 2401 2402 +1
==========================================
+ Hits 14395 14396 +1
+ Misses 3196 3190 -6
- Partials 1244 1252 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Could you add a test covering this? |
Signed-off-by: Tarun-kishore <[email protected]>
Signed-off-by: Tarun-kishore <[email protected]>
|
I've added unit test covering the parsing, but unit test cannot change |
Signed-off-by: Tarun-kishore <[email protected]>
This reverts commit cae2693. Signed-off-by: Tarun-kishore <[email protected]>
3 participants
Description
Fixed incorrect serialization of the
creationfield inExplainSMPolicy.toXContent(). The method was using.field()instead of.optionalField()for the nullablecreationfield, which is inconsistent with how thedeletionfield is handled and with the reference implementation inSMMetadata.toXContent().Changed line 39 in
ExplainSMPolicy.kt:.field(SMMetadata.CREATION_FIELD, it.creation).optionalField(SMMetadata.CREATION_FIELD, it.creation)This ensures proper handling of null creation workflow metadata in Explain API responses and aligns with the serialization behavior in
SMMetadata.Related Issues
Resolves #1506
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.